-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Creates Structure for Upload Requests #61
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, though I'm not sure I understand the API:
- Users request to upload N items
- Users get N upload URLs
- Users upload N files
- Users say "Hey, I uploaded " - One weakness here is that clients don't need to report the same number N of files here
- Users get back some ID? And the task starts running?
A more "bulletproof" API would be something like:
- Create a portfolio (or portfolio group, or process group whatever primitive files get attached to), get an ID back
- Request to upload files to that portfolio ID
- Can do step 2 as many times as is useful, can be batch or not batch, doesn't matter
- Mark files completed after they're uploaded. We could do this behind the scenes (as noted elsewhere, using blob storage events), but we could also just roll this validation step into step 5 below.
- Request to start the processing
From the web client's perspective, this whole process could be kicked off in one go once they select their files, it just makes the API slightly harder to misuse, intentionally or accidentally.
openapi/pacta.yaml
Outdated
CompletePortfolioUpload: | ||
type: object | ||
required: | ||
- incomplete_upload_ids |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Confusing name, since they are completed from the user's perspective, they're done uploading. I'd just call them upload_ids
, it's unambiguous here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to keep this as is - I think drift between terminology should be avoided when it's all internal only.
OK FWIW, I think this is mostly doing what you think it is, just with a few translations.
Does that make sense? |
The request is two step: (a) create the upload URLs and blobs, then after the assets are uploaded (b) kick off a process for parsing explicitly.
Sending the PR for the interface first - trying to get smaller ones going.